-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
S390x support #66
base: main
Are you sure you want to change the base?
S390x support #66
Conversation
Removed Goarch Signed-off-by: root <[email protected]>
use of buildx to build & push multi-architecture image Signed-off-by: root <[email protected]>
Install protoc for multi-architecture Signed-off-by: root <[email protected]>
2aa7104
to
53990f7
Compare
Signed-off-by: Matteo Mortari <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @modassarrana89 for a refreshed PR including most of the previous comments!
I've dded commit 8bcd792 as I believe necessary for the scripts/install_protoc.sh
script to correctly resolve the zip to download when used in a ARM based linux container.
The problem I see in the way this PR currently stands, is that the model-registry is not actually cross-compiled:
make image/buildx
# copy existing Dockerfile and insert --platform= into Dockerfile.cross, and preserve the original Dockerfile
sed -e '1 s/\(^FROM\)/FROM --platform=\$\{BUILDPLATFORM\}/; t' -e ' 1,// s//FROM --platform=\$\{BUILDPLATFORM\}/' Dockerfile > Dockerfile.cross
docker buildx create --name project-v3-builder
project-v3-builder
docker buildx use project-v3-builder
docker buildx build --platform=linux/arm64,linux/amd64,linux/s390x,linux/ppc64le --tag kubeflow/model-registry:main -f Dockerfile.cross .
[+] Building 239.2s (36/36) FINISHED docker-container:project-v3-builder
=> [internal] booting buildkit 2.1s
=> => pulling image moby/buildkit:buildx-stable-1 1.8s
=> => creating container buildx_buildkit_project-v3-builder0 0.3s
=> [internal] load build definition from Dockerfile.cross 0.0s
=> => transferring dockerfile: 1.47kB 0.0s
=> [linux/ppc64le internal] load metadata for registry.access.redhat.com/ubi8/ubi-minimal:8.8 3.6s
=> [linux/arm64 internal] load metadata for registry.access.redhat.com/ubi8/ubi-minimal:8.8 3.6s
=> [linux/amd64 internal] load metadata for registry.access.redhat.com/ubi8/ubi-minimal:8.8 3.8s
=> [linux/s390x internal] load metadata for registry.access.redhat.com/ubi8/ubi-minimal:8.8 3.8s
=> [linux/arm64 internal] load metadata for registry.access.redhat.com/ubi8/go-toolset:1.19 4.0s
=> [internal] load .dockerignore 0.0s
=> => transferring context: 52B 0.0s
=> [internal] load build context 0.2s
=> => transferring context: 6.79MB 0.2s
=> [linux/arm64 builder 1/19] FROM registry.access.redhat.com/ubi8/go-toolset:1.19@sha256:dd16d34f7e4fae45a620ba8fdde378d0853314e4e0f4998658fd81628b36f0d6 98.4s
=> => resolve registry.access.redhat.com/ubi8/go-toolset:1.19@sha256:dd16d34f7e4fae45a620ba8fdde378d0853314e4e0f4998658fd81628b36f0d6 0.0s
=> => sha256:b953f5a6908d8aa51e2200fe3aa4b40102eb758be1d56e7dd3bec2a9cb85662e 162.55MB / 162.55MB 87.9s
=> => sha256:b8104c5f40fdcf0549bf9ad8b9ea4e5d3fabcc446f920cb976986f255e884a42 143.61MB / 143.61MB 66.5s
=> => sha256:8550ec08a06cf30baf540ef95dcc7334958355ef556b38eca4fd67cd68de5af1 18.17MB / 18.17MB 13.8s
=> => sha256:e1bb0572465a9e03d7af5024abb36d7227b5bf133c448b54656d908982127874 76.88MB / 76.88MB 41.7s
=> => extracting sha256:e1bb0572465a9e03d7af5024abb36d7227b5bf133c448b54656d908982127874 1.0s
=> => extracting sha256:8550ec08a06cf30baf540ef95dcc7334958355ef556b38eca4fd67cd68de5af1 0.2s
=> => extracting sha256:b8104c5f40fdcf0549bf9ad8b9ea4e5d3fabcc446f920cb976986f255e884a42 1.7s
=> => extracting sha256:b953f5a6908d8aa51e2200fe3aa4b40102eb758be1d56e7dd3bec2a9cb85662e 1.6s
=> [linux/ppc64le stage-1 1/3] FROM registry.access.redhat.com/ubi8/ubi-minimal:8.8@sha256:b93deceb59a58588d5b16429fc47f98920f84740a1f2ed6454e33275f0701b59 27.2s
=> => resolve registry.access.redhat.com/ubi8/ubi-minimal:8.8@sha256:b93deceb59a58588d5b16429fc47f98920f84740a1f2ed6454e33275f0701b59 0.0s
=> => sha256:655f24e7656ea1c1a2711179ea0c6d6f7dac72ba7d2d3fd34c36d062f00f74d8 44.03MB / 44.03MB 26.7s
=> => extracting sha256:655f24e7656ea1c1a2711179ea0c6d6f7dac72ba7d2d3fd34c36d062f00f74d8 0.4s
=> [linux/arm64 stage-1 1/3] FROM registry.access.redhat.com/ubi8/ubi-minimal:8.8@sha256:b93deceb59a58588d5b16429fc47f98920f84740a1f2ed6454e33275f0701b59 38.1s
=> => resolve registry.access.redhat.com/ubi8/ubi-minimal:8.8@sha256:b93deceb59a58588d5b16429fc47f98920f84740a1f2ed6454e33275f0701b59 0.0s
=> => sha256:4da493486a8e6bed896d94989fd40199cacab25cef2022dc5b72dc9b0be61387 37.69MB / 37.69MB 37.7s
=> => extracting sha256:4da493486a8e6bed896d94989fd40199cacab25cef2022dc5b72dc9b0be61387 0.4s
=> [linux/amd64 stage-1 1/3] FROM registry.access.redhat.com/ubi8/ubi-minimal:8.8@sha256:b93deceb59a58588d5b16429fc47f98920f84740a1f2ed6454e33275f0701b59 36.9s
=> => resolve registry.access.redhat.com/ubi8/ubi-minimal:8.8@sha256:b93deceb59a58588d5b16429fc47f98920f84740a1f2ed6454e33275f0701b59 0.0s
=> => sha256:dc35b837139a95d1b9f7f7b0435a024a74ab972416bdc248f3f608c9f917a753 39.31MB / 39.31MB 36.5s
=> => extracting sha256:dc35b837139a95d1b9f7f7b0435a024a74ab972416bdc248f3f608c9f917a753 0.4s
=> [linux/s390x stage-1 1/3] FROM registry.access.redhat.com/ubi8/ubi-minimal:8.8@sha256:b93deceb59a58588d5b16429fc47f98920f84740a1f2ed6454e33275f0701b59 9.3s
=> => resolve registry.access.redhat.com/ubi8/ubi-minimal:8.8@sha256:b93deceb59a58588d5b16429fc47f98920f84740a1f2ed6454e33275f0701b59 0.0s
=> => sha256:8737e1657f05079caa8c726fb829e674f98ab5a3dd612e1bbd5a3b46b095e360 37.55MB / 37.55MB 8.9s
=> => extracting sha256:8737e1657f05079caa8c726fb829e674f98ab5a3dd612e1bbd5a3b46b095e360 0.4s
=> [linux/arm64 builder 2/19] WORKDIR /workspace 0.3s
=> [linux/arm64 builder 3/19] COPY [go.mod, go.sum, ./] 0.0s
=> [linux/arm64 builder 4/19] RUN go mod download 17.2s
=> [linux/arm64 builder 5/19] RUN yum remove -y nodejs npm 0.6s
=> [linux/arm64 builder 6/19] RUN yum module -y reset nodejs 2.7s
=> [linux/arm64 builder 7/19] RUN yum module -y enable nodejs:18 0.4s
=> [linux/arm64 builder 8/19] RUN yum install -y nodejs npm java-11 30.8s
=> [linux/arm64 builder 9/19] COPY [Makefile, main.go, .openapi-generator-ignore, openapitools.json, ./] 0.1s
=> [linux/arm64 builder 10/19] COPY .git/ .git/ 0.1s
=> [linux/arm64 builder 11/19] COPY cmd/ cmd/ 0.0s
=> [linux/arm64 builder 12/19] COPY api/ api/ 0.0s
=> [linux/arm64 builder 13/19] COPY internal/ internal/ 0.0s
=> [linux/arm64 builder 14/19] COPY scripts/ scripts/ 0.0s
=> [linux/arm64 builder 15/19] COPY pkg/ pkg/ 0.0s
=> [linux/arm64 builder 16/19] COPY patches/ patches/ 0.0s
=> [linux/arm64 builder 17/19] COPY templates/ templates/ 0.0s
=> [linux/arm64 builder 18/19] RUN make deps 55.0s
=> [linux/arm64 builder 19/19] RUN CGO_ENABLED=1 GOOS=linux make clean model-registry 27.1s
=> [linux/s390x stage-1 2/3] COPY --from=builder /workspace/model-registry . 0.0s
=> [linux/amd64 stage-1 2/3] COPY --from=builder /workspace/model-registry . 0.0s
=> [linux/ppc64le stage-1 2/3] COPY --from=builder /workspace/model-registry . 0.0s
=> [linux/arm64 stage-1 2/3] COPY --from=builder /workspace/model-registry . 0.0s
WARNING: No output specified with docker-container driver. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load
docker buildx rm project-v3-builder
project-v3-builder removed
rm Dockerfile.cross
In other words: it looks to me model registry get compiled for the platform the build is currently on (in my screenshot, arm), and then the binary copied to linux/arm64,linux/amd64,linux/s390x,linux/ppc64le
, which is unlikely to work.
Further, it looks to me cross-compilation would require something ~like this in the dockerfile:
ARG TARGETOS TARGETARCH
RUN CGO_ENABLED=1 GOOS=$TARGETOS GOARCH=$TARGETARCH make clean model-registry
but currently would fail with:
go: cannot install cross-compiled binaries when GOBIN is set
which is used as part of the Makefile.
I believe solutions could be along the lines of:
- rework the build to avoid gobin IFF all the remainder buildx problems are solved after accounting for the above
- use buildx for emulation rather than cross-compilation
- other solution?
I believe a proper reproducible cross-platform build is needed, advise if you believe I might have missed on anything and how planning to solve for the mentioned issues.
Let me quickly test , once & get back on this |
Added TargetOS and TARGETARCH
Below Github action works fine for building & pushing multi - arch image
If ok , we can update the github workflow for build and push to use this action & then it will automatically solves our problem. |
Not seeing / nor understood how the new commit would solve for:
|
* Refactor openapi definition * Add name query parameter for model_version url, fix Artifact oneOf definition * Added sortOrder query param * Fix allOf composites for BaseArtifactUpdate and ModelArtifactUpdate * Add model-serving metadata resources, openapi-generator-cli, and generated model, server, and proxy cmd * Fix allOf types for InferenceServiceCreate and InferenceServiceUpdate
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@tarilabs Below Github action should works fine for building & pushing multi - arch image. Need to verify once . We can use this inplace of the current scripts for multi-arch image build procedure . If you guys are. fine with Github workflow change , I can quickly verify this approach & let you know the update.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
We are looking into removing the GOBIN and local build toolchain, so to support more general multi-arch support, per KF 1.10 roadmap: #175 We can keep this PR open, to revise this as we will rampup on those items. |
Ok Great. But may i know whats the deadline would be |
@tarilabs it would be great if you can share some update on GOBIN removal and local build toolchain. |
Sure! There is also another observation that generated source code are re-generated in the docker build phase itself, which is less ideal especially considering we have them already under VCS here in this git repo, in order to point to the actual sources for the bin/image we ship. We have some contributors actively working on this front for which we can expect some further update, per PRs already iterated on, or being currently worked on right now. edit: with more recent changes, we just discovered gobin looks currently not an impediment to crosscompilation, so we're now looking to keep it (hence not drop its current usage). More updates to follow. |
we have discussed previously that cross compilation is creating problem with current GOBIN setup. If GOBIN is not an issue , are you saying that cross-compilation will work |
as mentioned in #66 (comment), gobin looks currently not an impediment to crosscompilation, so we're now looking to keep (gobin usage) more updates to follow |
Multi-architecture support for model-registry
Description
Updated Dockerfile
Updated Makefile to use buildx for multi-arch image
Updated install_protoc scripts for multi-architecture